Skip to content

sycl: add CONCAT operator support #16047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Nov 6, 2025
Merged

sycl: add CONCAT operator support #16047

merged 11 commits into from
Nov 6, 2025
+73 −66

Conversation

@ye-NX
Copy link
Contributor

@ye-NX ye-NX commented Sep 17, 2025 •
edited
Loading

Title:
sycl: add CONCAT support for GGML_TYPE_I16 and GGML_TYPE_I32

Body:

  • This PR generalizes the SYCL backend CONCAT operator to support GGML_TYPE_I16 and GGML_TYPE_I32.

  • Refactors float-only kernels into templated kernels (T) for dim0/1/2 and the non-contiguous fallback.

  • Replaces /4 with elem_size(dst->type) to ensure correct indexing for all types.

  • Adds a type switch in ggml_sycl_op_concat to invoke concat_impl_sycl for F32, I16, and I32.

  • No API changes; no behavior changes for other backends.

Testing

Built with -DGGML_SYCL=ON and verified correctness for F32, I16, I32 across dim 0/1/2/3 with both contiguous and non-contiguous inputs.

test-backend-ops passes for CONCAT.

@github-actions github-actions bot added testing Everything test related ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language labels Sep 17, 2025
ngxson
ngxson requested changes Sep 19, 2025
View reviewed changes
@ye-NX ye-NX force-pushed the ye/sycl-concat-additional-support branch from d889219 to 8d6da48 Compare September 21, 2025 07:24
@ye-NX
Copy link
Contributor Author

ye-NX commented Sep 21, 2025

You're right, the branch was based on an outdated master.
I’ve rebased onto upstream/master and force-pushed.

@ye-NX ye-NX mentioned this pull request Oct 16, 2025
Merged
@NeoZhangJianyu
Copy link
Collaborator

NeoZhangJianyu commented Oct 17, 2025

@ye-NX
I think the PR should rebased to remove the unimpacted file, like ggufxxx.
The test-backend-op.cpp should be updated to restore the change of other legacy UT cases, except CONCAT OP.

@ye-NX ye-NX force-pushed the ye/sycl-concat-additional-support branch from 8d6da48 to aad3d99 Compare October 19, 2025 05:49
@ye-NX ye-NX requested a review from slaren as a code owner October 19, 2025 05:49
@ye-NX
Copy link
Contributor Author

ye-NX commented Oct 19, 2025

@NeoZhangJianyu
Thank you for reviewing! I’ve addressed the feedback:
– Rebased on the latest master and removed unrelated files (gguf*, b6524/b6527).
– Restored legacy tests in tests/test-backend-ops.cpp, keeping only the CONCAT changes.

Could you please take another look?

NeoZhangJianyu
NeoZhangJianyu reviewed Oct 20, 2025
View reviewed changes
Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test-backend-ops.cpp has many change.
It will impact other backend.

Please only add new cases, instead of changing or deleting existed cases.

Thank you!

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 21, 2025
@github-actions github-actions bot added examples python python script changes devops improvements to build systems and github actions server labels Oct 21, 2025
@github-actions github-actions bot added Nvidia GPU Issues specific to Nvidia GPUs Vulkan Issues specific to the Vulkan backend OpenCL Issues specific to the OpenCL backend labels Oct 21, 2025
@NeoZhangJianyu
Copy link
Collaborator

NeoZhangJianyu commented Oct 22, 2025

@ye-NX
There are 76 files to be changed. :)
Maybe you need to rebase your branch.

@max-krasnyansky
Copy link
Collaborator

max-krasnyansky commented Oct 22, 2025

@ye-NX There are 76 files to be changed. :) Maybe you need to rebase your branch.

Yep. Looks like ggml-opencl changes got mixed in due to incorrect/incomplete rebase.

@allozaur allozaur removed their request for review October 22, 2025 16:22
NeoZhangJianyu
NeoZhangJianyu reviewed Oct 23, 2025
View reviewed changes
Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only 4 files needed.
But there are 6 empty files in this PR.
Could remove them?

image

@CISC CISC removed documentation Improvements or additions to documentation testing Everything test related Nvidia GPU Issues specific to Nvidia GPUs Vulkan Issues specific to the Vulkan backend examples python python script changes devops improvements to build systems and github actions server OpenCL Issues specific to the OpenCL backend labels Oct 30, 2025
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 30, 2025
CISC
CISC reviewed Oct 30, 2025
View reviewed changes
@CISC CISC requested review from NeoZhangJianyu and ngxson October 30, 2025 10:30
@ye-NX
Copy link
Contributor Author

ye-NX commented Oct 30, 2025

@CISC , @NeoZhangJianyu
Can someone check if everything is okay now? Thanks!

NeoZhangJianyu
NeoZhangJianyu approved these changes Oct 31, 2025
View reviewed changes
Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

@NeoZhangJianyu
Copy link
Collaborator

NeoZhangJianyu commented Oct 31, 2025

@ngxson
Please review again!
It's fixed for your comment.

@CISC
Copy link
Collaborator

CISC commented Nov 2, 2025

@ngxson Please unblock.

@ngxson ngxson merged commit 9d7c518 into ggml-org:master Nov 6, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CISC CISC CISC left review comments

@ngxson ngxson ngxson approved these changes

@NeoZhangJianyu NeoZhangJianyu NeoZhangJianyu approved these changes

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ye-NX @NeoZhangJianyu @max-krasnyansky @CISC @ngxson